Skip to content

fix(openaiShim): unconditionally set reasoning_content for DeepSeek V…#918

Open
skeeminator wants to merge 1 commit intoGitlawb:mainfrom
skeeminator:fix/deepseek-v4-reasoning-content
Open

fix(openaiShim): unconditionally set reasoning_content for DeepSeek V…#918
skeeminator wants to merge 1 commit intoGitlawb:mainfrom
skeeminator:fix/deepseek-v4-reasoning-content

Conversation

@skeeminator
Copy link
Copy Markdown

@skeeminator skeeminator commented Apr 27, 2026

Summary

  • DeepSeek V4 (and similar reasoning providers) require reasoning_content
    present on every assistant message in the request history when thinking
    mode is active. The API accepts empty string — the field just needs to
    exist.
  • Prior guard only set reasoning_content when thinkingText.trim().length > 0,
    so tool-call turns and edge cases (synthetic interrupts, non-array content)
    sent assistant messages without the field → 400.
  • Fix: always set reasoning_content when preserveReasoningContent is true,
    falling back to "" when thinking text is missing. Two additional sites
    (coalescing interrupt, string-content else branch) now also emit the field.

Impact

  • user-facing: fixes API Error: 400 {"error":{"message":"The reasoning_content in the thinking mode must be passed back to the API."}}
    for DeepSeek V4 models (deepseek-v4-pro, deepseek-v4-flash) against
    api.deepseek.com. Tool-call rounds and plain multi-turn now work.
  • developer/maintainer: gated behind existing preserveReasoningContent
    flag. Non-DeepSeek providers (OpenAI, Gemini, Mistral, etc.) unaffected.

Testing

  • bun run build
  • bun run smoke
  • focused tests: bun test src/services/api/openaiShim.test.ts
  • live verification: deepseek-v4-pro against api.deepseek.com
    multi-turn tool-call round completed without 400

Notes

  • provider/model path tested: DeepSeek (api.deepseek.com) with
    deepseek-v4-pro
  • follow-up work or known limitations: none

…4 thinking mode

DeepSeek V4 (and similar reasoning providers) require the reasoning_content
field to be present on every assistant message when thinking mode is
enabled — even as an empty string. The previous guard only set it when
thinking text was non-empty (typeof === 'string' && trim().length > 0),
causing 400 errors on multi-turn tool-call rounds.

Three sites fixed in convertMessages():
- Assistant messages with thinking blocks: now always set reasoning_content
  (fallback to empty string when thinking text is absent)
- Coalescing synthetic interrupt message: added reasoning_content: ''
- String-content fallback branch: added reasoning_content: '' when
  preserveReasoningContent is true
@jatmn
Copy link
Copy Markdown
Collaborator

jatmn commented Apr 27, 2026

Will be impacted by #910

@kevincodex1
Copy link
Copy Markdown
Contributor

lets wait for #910

Copy link
Copy Markdown
Collaborator

@gnanam1990 gnanam1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tight, surgical fix. Gating behind the existing preserveReasoningContent flag keeps non-DeepSeek providers unaffected, and the three-site coverage in convertMessages matches the failure modes described. Clear repro against api.deepseek.com is appreciated. LGTM.

Optional follow-up: a unit test for the preserveReasoningContent && !thinkingText path would harden this against future regressions.

@AxDSan
Copy link
Copy Markdown

AxDSan commented May 1, 2026

Omgosh, deepseek is unusable right now in OpenClaude because of this :( Will keep following until there is a merge! thanks!

@jatmn
Copy link
Copy Markdown
Collaborator

jatmn commented May 2, 2026

#910 merged, please rebase and fix conflicts.

@kevincodex1
Copy link
Copy Markdown
Contributor

hello @skeeminator kindly rebase and fix conflicts this is good to merge .

Copy link
Copy Markdown
Collaborator

@Vasanthdev2004 Vasanthdev2004 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Targeted maintainer triage review of the current head ($short).

Verdict: Needs changes

Blocking issue:

  1. GitHub reports this branch as DIRTY / conflicting with main, so it cannot be merged or final-approved as-is. Please rebase or merge latest main, resolve the conflicts, and rerun the relevant checks.

I did not do a full code review because the current branch state is not mergeable. Happy to re-review once the branch is clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants